#63 - Add Visual affordance for required photo upload on /setup page#67
#63 - Add Visual affordance for required photo upload on /setup page#67AndrewHUNGNguyen wants to merge 4 commits intoDEVxNetwork:mainfrom
Conversation
|
@samholmes when you are free, this PR will be ready for you and hope this will be good by Saturday's monthly meetup 🙂 |
samholmes
left a comment
samholmes
left a comment
There was a problem hiding this comment.
These requested changes are minor but worthwhile. Great work!
|
|
||
| const FieldError = styled.p` | ||
| color: #f87171; | ||
| font-size: 0.75rem; |
There was a problem hiding this comment.
font-size here is not consistent with font-size in TextInput.
app/components/Nametag.tsx
Outdated
There was a problem hiding this comment.
Consider defining a color --error-color: #f87171 in :root declaration in globals.css. This way we have a consistent color as part of our theming.
app/setup/page.tsx
Outdated
There was a problem hiding this comment.
font-size 0.75rem to be consistent with the other FieldError in the nametag?
app/setup/page.tsx
Outdated
There was a problem hiding this comment.
Instead of inlining a new handler function, move your implementation to the handleImageUpload handler. Rework your changes into that existing handler.
app/setup/page.tsx
Outdated
There was a problem hiding this comment.
Instead of inlining a new handler function, move your implementation to the a handleDataChange handler.
app/setup/page.tsx
Outdated
There was a problem hiding this comment.
Move this to a handleHandleChange. Hah, that's a funny name. Typical naming convention for handler functions is handle[semantic][event], in this case the semantic is "handle" because that's what we call the username, and the event is "Change" because it's for "onChange". Hence the weird sounding name.
app/components/TextInput.tsx
Outdated
There was a problem hiding this comment.
Might as well change the ternary here to an if statement while we're at it.
app/components/TextInput.tsx
Outdated
There was a problem hiding this comment.
Consider replacing ternary to if statement
app/components/Nametag.tsx
Outdated
There was a problem hiding this comment.
Define a handlePhotoFrameClick handler function and implement the logic there.
AndrewHUNGNguyen
force-pushed
the
#63-Add-Vis-Afford-Photo-Upload
branch
from
67d1f7a to
83d3a8b
Compare
2 participants
Created red borders for required fields (See below screenshot):
Ensured that following original behaviors are not impacted: